fix(server): unify the license headers #2438

msgui · 2024-02-01T15:27:11Z

Purpose of the PR

subtask of [Summary] Enhance code style warnings in server #2435

Main Changes

Verifying these changes

Trivial rework / code cleanup without any test coverage. (No Need)
Already covered by existing tests, such as (please modify tests here).
Need tests and can be verified as follows:
- xxx

Does this PR potentially affect the following parts?

Documentation Status

Doc - TODO
Doc - Done
Doc - No Need

codecov · 2024-02-01T15:35:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d4532c) 66.23% compared to head (9ed3749) 61.39%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2438      +/-   ##
============================================
- Coverage     66.23%   61.39%   -4.84%     
+ Complexity      828      488     -340     
============================================
  Files           511      511              
  Lines         42597    42598       +1     
  Branches       5942     5942              
============================================
- Hits          28215    26154    -2061     
- Misses        11566    13759    +2193     
+ Partials       2816     2685     -131

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

imbajin

fine, wonder to know all the files header are updated?

msgui · 2024-02-02T02:34:55Z

fine, wonder to know all the files header are updated?

This PR solely updates the license header in hg-server/hg-api.

imbajin · 2024-02-02T03:05:56Z

fine, wonder to know all the files header are updated?

This PR solely updates the license header in hg-server/hg-api.

Get it, license header could update together (due to it is almost a completely repetitive task, just take a few samples for inspection and confirmation, no need to check them one by one)

VGalaxies · 2024-02-02T07:40:08Z

@msgui I noticed that some configuration files, such as hugegraph-server/hugegraph-test/src/main/resources/hugegraph.properties, do not have corresponding modified license headers. Is this expected?

msgui · 2024-02-02T15:39:26Z

@msgui I noticed that some configuration files, such as hugegraph-server/hugegraph-test/src/main/resources/hugegraph.properties, do not have corresponding modified license headers. Is this expected?

thanks for the reminder

msgui · 2024-02-02T15:44:53Z

this commit includes files of all formats for easy review.

👉:47e2fe24976efe092fd6f944444be00ba142cba5

VGalaxies

PTAL~

It seems reasonable to consider updating the license header for the entire repository, not just under hugegraph-server.
For the license headers only under hugegraph-server, there still seem to be some missed files. I have conducted a preliminary count (just a reminder):

(expected: 791, actual: 783)

VGalaxies · 2024-02-03T04:37:43Z

Also, these extra blank lines here can be removed, it seems?

(compared to https://github.com/apache/incubator-hugegraph/wiki/HugeGraph-Code-Style-Guide)

msgui · 2024-02-03T05:58:52Z

PTAL~

It seems reasonable to consider updating the license header for the entire repository, not just under hugegraph-server.

For the license headers only under hugegraph-server, there still seem to be some missed files. I have conducted a preliminary count (just a reminder):

(expected: 791, actual: 783)

Thank you for the reminder. The license header format in the remaining files is already correct, so no further corrections are needed

eg: hugegraph-server/Dockerfile

VGalaxies

Great work! I have performed verification locally following the steps in Project Migration/Package Name Change + IDEA Configuration Instructions (Complete Version), and it meets expectations.

@msgui Consider supplementing the detailed steps for updating copyright to the above document, explaining how to configure IDEA from scratch and how to update copyright with a single click (Because I encountered some unclear points during the configuration process, such as how to match the files that need updating, the relevant content in the above document is somewhat scattered. It might be beneficial to consolidate it together)

VGalaxies · 2024-02-03T07:23:36Z

@imbajin could also check again~

Additionally, I believe that performing the license header check in the current CI (licence-checker.yml) seems somewhat delayed. It might be worth considering moving it to the verify stage. This way, developers can discover related errors earlier during local builds.

imbajin · 2024-02-04T08:11:18Z

@imbajin could also check again~

Additionally, I believe that performing the license header check in the current CI (licence-checker.yml) seems somewhat delayed. It might be worth considering moving it to the verify stage. This way, developers can discover related errors earlier during local builds.

Good suggestion, could mark it as a TODO (enhance our CI to check/validate style problems)

liuxiaocs7 · 2024-02-04T08:43:15Z

This tool hawkeye may help license header check and format?

imbajin

TODOs:

keep the LICENSE header from refer code (revert/exclude it by reg?)
rearrange the commit range
keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)
remove the header in org.apache.tinkerpop.gremlin.server.OpProcessor or GremlinPlugin (pure text file)

VGalaxies

It seems that some files license headers are inconsistent with those in https://github.com/apache/incubator-hugegraph/wiki/HugeGraph-Code-Style-Guide.

TODOs:

keep the LICENSE header from refer code (revert/exclude it by reg?)

rearrange the commit range

keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)

remove the header in org.apache.tinkerpop.gremlin.server.OpProcessor or GremlinPlugin (pure text file)

Also, do the TODOs here need to be addressed in this PR? @imbajin

imbajin

Almost have checked all the files already (Also finished all the TODOS now)

…bator-hugegraph into CORRECT-LICENSE-API

VGalaxies

LGTM~

subtask of #2435 --------- Co-authored-by: imbajin <[email protected]> Co-authored-by: VGalaxies <[email protected]>

This reverts commit 7586779.

VGalaxies · 2024-03-02T04:05:38Z

keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)

@imbajin Is it only for cleaning the license header in properties files in user configuration files here? I found that some other user configuration files have license headers.

Also, do properties files in the conf-raft* directory need to have their license headers cleaned?

imbajin · 2024-03-02T13:29:28Z

keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)

@imbajin Is it only for cleaning the license header in properties files in user configuration files here? I found that some other user configuration files have license headers.

@VGalaxies
The current judgment points are:

If a configuration file is not frequently used/modified by users, following ASF's suggestion may be better to add a file header to reduce disputes
Otherwise(In HG), considering that there are already many configuration files and the content is relatively long, the commonly used configurations by users are not separately added to avoid affecting the user experience
(like point 1) Configuration files from testing/and other modules could also add headers (although not required)

Also, do properties files in the conf-raft* directory need to have their license headers cleaned?

So these places seem to fit the 1st & 3rd points and can preserve license headers.

msgui and others added 2 commits February 1, 2024 21:50

Update hugegraph-style.xml

f95e2c8

Merge branch 'apache:master' into master

01f4ff0

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. api Changes of API bug Something isn't working labels Feb 1, 2024

msgui mentioned this pull request Feb 1, 2024

[Summary] Enhance code style warnings in server #2435

Closed

18 tasks

imbajin reviewed Feb 1, 2024

View reviewed changes

msgui changed the title ~~fix: Correct License HG-Server-API~~ fix: Correct License HG-Server Feb 2, 2024

msgui requested a review from imbajin February 2, 2024 03:14

dosubot bot modified the milestones: 1.3.0, 1.5.0 Feb 2, 2024

msgui changed the title ~~fix: Correct License HG-Server~~ fix: correct License hg-server Feb 2, 2024

VGalaxies requested changes Feb 3, 2024

View reviewed changes

msgui force-pushed the correct-listen-header branch from ec696fc to 3321852 Compare February 3, 2024 05:49

VGalaxies previously approved these changes Feb 3, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 3, 2024

imbajin reviewed Feb 5, 2024

View reviewed changes

msgui dismissed VGalaxies’s stale review via 01f4ff0 February 5, 2024 05:02

msgui added 2 commits February 5, 2024 16:56

format core

0e108cb

format the remaining modules

87c1f89

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 5, 2024

msgui requested review from imbajin and VGalaxies February 6, 2024 04:36

VGalaxies requested changes Feb 6, 2024

View reviewed changes

tiny fix

c745b81

imbajin previously approved these changes Feb 6, 2024

View reviewed changes

double check and fix

2cc39d3

VGalaxies dismissed imbajin’s stale review via 2cc39d3 February 6, 2024 10:44

msgui added 5 commits February 6, 2024 18:48

fix api

42b179b

fix core

53d4dbb

fix test

b82e30d

fix the remaining modules

4616f0d

Merge branch 'correct-listen-header' of https://github.com/msgui/incu…

3d3b876

…bator-hugegraph into CORRECT-LICENSE-API

msgui requested review from VGalaxies and imbajin February 6, 2024 11:03

minor fix

9ed3749

VGalaxies approved these changes Feb 6, 2024

View reviewed changes

imbajin approved these changes Feb 7, 2024

View reviewed changes

imbajin changed the title ~~fix: correct License hg-server~~ fix(server): unify the license headers Feb 7, 2024

imbajin merged commit 7586779 into apache:master Feb 7, 2024
20 of 21 checks passed

VGalaxies added a commit that referenced this pull request Feb 7, 2024

fix(server): unify the license headers (#2438)

92f2ecb

subtask of #2435 --------- Co-authored-by: imbajin <[email protected]> Co-authored-by: VGalaxies <[email protected]>

VGalaxies mentioned this pull request Feb 7, 2024

fix(server): unify license header for protobuf file #2448

Merged

returnToInnocence added a commit to msgui/incubator-hugegraph that referenced this pull request Feb 13, 2024

Revert "fix(server): unify the license headers (apache#2438)"

e201b67

This reverts commit 7586779.

msgui deleted the correct-listen-header branch March 15, 2024 04:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(server): unify the license headers #2438

fix(server): unify the license headers #2438

msgui commented Feb 1, 2024 •

edited

Loading

codecov bot commented Feb 1, 2024 •

edited

Loading

imbajin left a comment •

edited

Loading

msgui commented Feb 2, 2024

imbajin commented Feb 2, 2024 •

edited

Loading

VGalaxies commented Feb 2, 2024

msgui commented Feb 2, 2024 •

edited

Loading

msgui commented Feb 2, 2024

VGalaxies left a comment •

edited

Loading

VGalaxies commented Feb 3, 2024

msgui commented Feb 3, 2024 •

edited

Loading

VGalaxies left a comment

VGalaxies commented Feb 3, 2024

imbajin commented Feb 4, 2024

liuxiaocs7 commented Feb 4, 2024

imbajin left a comment •

edited

Loading

VGalaxies left a comment •

edited

Loading

imbajin left a comment

VGalaxies left a comment

VGalaxies commented Mar 2, 2024

imbajin commented Mar 2, 2024 •

edited

Loading

fix(server): unify the license headers #2438

fix(server): unify the license headers #2438

Conversation

msgui commented Feb 1, 2024 • edited Loading

Purpose of the PR

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

codecov bot commented Feb 1, 2024 • edited Loading

Codecov Report

imbajin left a comment • edited Loading

Choose a reason for hiding this comment

msgui commented Feb 2, 2024

imbajin commented Feb 2, 2024 • edited Loading

VGalaxies commented Feb 2, 2024

msgui commented Feb 2, 2024 • edited Loading

msgui commented Feb 2, 2024

VGalaxies left a comment • edited Loading

Choose a reason for hiding this comment

VGalaxies commented Feb 3, 2024

msgui commented Feb 3, 2024 • edited Loading

VGalaxies left a comment

Choose a reason for hiding this comment

VGalaxies commented Feb 3, 2024

imbajin commented Feb 4, 2024

liuxiaocs7 commented Feb 4, 2024

imbajin left a comment • edited Loading

Choose a reason for hiding this comment

VGalaxies left a comment • edited Loading

Choose a reason for hiding this comment

imbajin left a comment

Choose a reason for hiding this comment

VGalaxies left a comment

Choose a reason for hiding this comment

VGalaxies commented Mar 2, 2024

imbajin commented Mar 2, 2024 • edited Loading

msgui commented Feb 1, 2024 •

edited

Loading

codecov bot commented Feb 1, 2024 •

edited

Loading

imbajin left a comment •

edited

Loading

imbajin commented Feb 2, 2024 •

edited

Loading

msgui commented Feb 2, 2024 •

edited

Loading

VGalaxies left a comment •

edited

Loading

msgui commented Feb 3, 2024 •

edited

Loading

imbajin left a comment •

edited

Loading

VGalaxies left a comment •

edited

Loading

imbajin commented Mar 2, 2024 •

edited

Loading